Skip to content

add evaluateFunctionExit for backward analyzer in monotone-analyzer - #7547

Merged
tlively merged 1 commit into
WebAssembly:mainfrom
HerrCai0907:binaryen-backport
May 7, 2025
Merged

add evaluateFunctionExit for backward analyzer in monotone-analyzer#7547
tlively merged 1 commit into
WebAssembly:mainfrom
HerrCai0907:binaryen-backport

Conversation

@HerrCai0907

Copy link
Copy Markdown
Contributor

For backward analyzer, it is helpful to have a function to init state in the "entry point" (exit block).

@kripken
kripken requested a review from tlively April 24, 2025 23:03
@tlively

tlively commented Apr 24, 2025

Copy link
Copy Markdown
Member

Makes sense, but would it be possible to add some usage somewhere to avoid this getting deleted as dead code in the future?

@HerrCai0907

Copy link
Copy Markdown
Contributor Author

I am writing our own pass to analyze the opcode between local.get and corresponding really uses (ignore block / if / loop...). It needs to handle uses as return value from operand stack which needs this functional.
In current binaryen, I can only find LivenessTransferFunction is backward analysis but it does not need this function.

@tlively

tlively commented Apr 25, 2025

Copy link
Copy Markdown
Member

Would you be interested in upstreaming the pass you're writing? If not, and you're maintaining downstream patches, what's the benefit of having this function upstream?

@HerrCai0907

Copy link
Copy Markdown
Contributor Author

Would you be interested in upstreaming the pass you're writing?

I am not sure the attitude of the bianryen community to frontend language specific pass. I guess it only welcomes the common part.

what's the benefit of having this function upstream?

To make the lattice based analysis framework more complete. If it is not needed, feel free to close this PR.

@tlively

tlively commented Apr 29, 2025

Copy link
Copy Markdown
Member

There is some precedent for having frontend-specific passes in Binaryen. We have J2CLOpts.cpp, which is specific to the j2wasm compiler, for example. We could consider adding more frontend-specific passes on a case-by-case basis. This would have the benefit to you of making sure your pass isn't broken if we refactor the static analysis framework and would also make sure that we have test coverage for the new method in this PR.

Alternatively we can just land this PR as-is, but be aware that future refactors might break your downstream pass unexpectedly. This isn't meant to be a stable, external-facing API.

@HerrCai0907

Copy link
Copy Markdown
Contributor Author

I prefer to merge it as current PR.
What I am doing is a better pass to implement GC for wasm1.0 (replacement of https://github.com/AssemblyScript/assemblyscript/blob/main/src/passes/shadowstack.ts). It is quick complex and high depends on some assumption in assemblyscript, so I don't find a good way to extract common part to upstream it.

but be aware that future refactors might break your downstream pass unexpectedly

It is fine since normally only C api is stable.

@tlively tlively left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this seems fine to merge, then.

@tlively
tlively merged commit 9e6bae0 into WebAssembly:main May 7, 2025
@HerrCai0907
HerrCai0907 deleted the binaryen-backport branch June 10, 2025 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants